Respect yanks in the registry
authorAlex Crichton <alex@alexcrichton.com>
Thu, 23 Oct 2014 19:21:08 +0000 (12:21 -0700)
committerAlex Crichton <alex@alexcrichton.com>
Mon, 27 Oct 2014 19:40:23 +0000 (12:40 -0700)
In general the semantics of a yank are that the code itself is not removed from
the registry, but rather packages are no longer allowed to depend on the
version. A yank is normally done to remove broken code or perhaps secrets, but
actually deleting code means that all packages depending on the yanked version
all of a sudden break. For these reasons a yank does not actually delete code,
but only flags the version as yanked in the index.

Yanked packages are therefore able to be depended upon if a lockfile points at a
yanked version, but are not allowed to become new dependencies of packages.

Implementation-wise, the following changes were made:

* SourceIds originating from a lockfile for registries will have a precise
  version listed (just a generic string "locked")
* Dependencies which use precise source ids are allowed to read yanked versions
* Dependencies without a precise source id are not allowed to use yanked
  versions

When using a lockfile (or a previous instance of resolve), all operations will
rewrite dependencies to have the precise source ids where applicable, meaning
the locked versions have access to yanked versions, but the unlocked versions do
not.

src/cargo/core/source.rs
src/cargo/sources/registry.rs
tests/support/registry.rs
tests/test_cargo_registry.rs

index 08a105b0e601d6c9e49bbea4308a7212dbe1c70d..9d3edf68ad5e84a8ee5cf95f4b7b347b231449ba 100644 (file)
@@ -114,7 +114,8 @@ impl SourceId {
             },
             "registry" => {
                 let url = url.to_url().unwrap();
-                SourceId::for_registry(&url)
+                SourceId::new(RegistryKind, url)
+                         .with_precise(Some("locked".to_string()))
             }
             "path" => SourceId::for_path(&Path::new(url.slice_from(5))).unwrap(),
             _ => fail!("Unsupported serialized SourceId")
index 9581cde3b1cb20f4b0b334c8f75cb0fb4467a06e..4152d9dbcf0d2346011acb777d2412134fcece95 100644 (file)
@@ -43,6 +43,7 @@ struct RegistryPackage {
     deps: Vec<RegistryDependency>,
     features: HashMap<String, Vec<String>>,
     cksum: String,
+    yanked: Option<bool>,
 }
 
 #[deriving(Decodable)]
@@ -186,9 +187,12 @@ impl<'a, 'b> RegistrySource<'a, 'b> {
 
     /// Parse a line from the registry's index file into a Summary for a
     /// package.
-    fn parse_registry_package(&mut self, line: &str) -> CargoResult<Summary> {
+    ///
+    /// The returned boolean is whether or not the summary has been yanked.
+    fn parse_registry_package(&mut self, line: &str)
+                              -> CargoResult<(Summary, bool)> {
         let RegistryPackage {
-            name, vers, cksum, deps, features
+            name, vers, cksum, deps, features, yanked
         } = try!(json::decode::<RegistryPackage>(line));
         let pkgid = try!(PackageId::new(name.as_slice(),
                                         vers.as_slice(),
@@ -198,7 +202,7 @@ impl<'a, 'b> RegistrySource<'a, 'b> {
         }).collect();
         let deps = try!(deps);
         self.hashes.insert((name, vers), cksum);
-        Summary::new(pkgid, deps, features)
+        Ok((try!(Summary::new(pkgid, deps, features)), yanked.unwrap_or(false)))
     }
 
     /// Converts an encoded dependency in the registry to a cargo dependency
@@ -234,14 +238,17 @@ impl<'a, 'b> Registry for RegistrySource<'a, 'b> {
             Err(..) => return Ok(Vec::new()),
         };
 
-        let ret: CargoResult<Vec<Summary>>;
+        let ret: CargoResult<Vec<(Summary, bool)>>;
         ret = contents.as_slice().lines().filter(|l| l.trim().len() > 0)
                       .map(|l| self.parse_registry_package(l))
                       .collect();
-        let mut summaries = try!(ret.chain_error(|| {
+        let summaries = try!(ret.chain_error(|| {
             internal(format!("Failed to parse registry's information for: {}",
                              dep.get_name()))
         }));
+        let mut summaries = summaries.into_iter().filter(|&(_, yanked)| {
+            dep.get_source_id().get_precise().is_some() || !yanked
+        }).map(|(summary, _)| summary).collect::<Vec<_>>();
         summaries.query(dep)
     }
 }
index c25953e60696bdd38b1162816e708f908e05dbd5..bac46784df7429ad304140b7cc98ecb1bd8b85fc 100644 (file)
@@ -68,9 +68,14 @@ pub fn mock_archive_dst(name: &str, version: &str) -> Path {
 }
 
 pub fn mock_pkg(name: &str, version: &str, deps: &[(&str, &str)]) {
+    mock_pkg_yank(name, version, deps, false)
+}
+
+pub fn mock_pkg_yank(name: &str, version: &str, deps: &[(&str, &str)],
+                     yanked: bool) {
     mock_archive(name, version, deps);
     let c = File::open(&mock_archive_dst(name, version)).read_to_end().unwrap();
-    let line = pkg(name, version, deps, cksum(c.as_slice()).as_slice());
+    let line = pkg(name, version, deps, cksum(c.as_slice()).as_slice(), yanked);
 
     let file = match name.len() {
         1 => format!("1/{}", name),
@@ -102,10 +107,13 @@ pub fn publish(file: &str, line: &str) {
                 [&parent]).unwrap();
 }
 
-pub fn pkg(name: &str, vers: &str, deps: &[(&str, &str)], cksum: &str) -> String {
+pub fn pkg(name: &str, vers: &str, deps: &[(&str, &str)], cksum: &str,
+           yanked: bool) -> String {
     let deps = deps.iter().map(|&(a, b)| dep(a, b)).collect::<Vec<String>>();
-    format!(r#"{{"name":"{}","vers":"{}","deps":{},"cksum":"{}","features":{{}}}}"#,
-            name, vers, deps, cksum)
+    format!("{{\"name\":\"{}\",\"vers\":\"{}\",\
+               \"deps\":{},\"cksum\":\"{}\",\"features\":{{}},\
+               \"yanked\":{}}}",
+            name, vers, deps, cksum, yanked)
 }
 
 pub fn dep(name: &str, req: &str) -> String {
index e72dd6f2ebafb179cd69731f4f6291801e54a4fd..27e34d6690fad6362d09d6c7b771deb52b1f1237 100644 (file)
@@ -1,4 +1,4 @@
-use std::io::File;
+use std::io::{fs, File};
 
 use support::{project, execs, cargo_dir};
 use support::{UPDATING, DOWNLOADING, COMPILING, PACKAGING, VERIFYING};
@@ -291,3 +291,96 @@ test!(lockfile_locks_transitively {
 {updating} registry `[..]`
 ", updating = UPDATING).as_slice()));
 })
+
+test!(yanks_are_not_used {
+    let p = project("foo")
+        .file("Cargo.toml", r#"
+            [project]
+            name = "foo"
+            version = "0.0.1"
+            authors = []
+
+            [dependencies]
+            bar = "*"
+        "#)
+        .file("src/main.rs", "fn main() {}");
+    p.build();
+
+    r::mock_pkg("baz", "0.0.1", []);
+    r::mock_pkg_yank("baz", "0.0.2", [], true);
+    r::mock_pkg("bar", "0.0.1", [("baz", "*")]);
+    r::mock_pkg_yank("bar", "0.0.2", [("baz", "*")], true);
+
+    assert_that(p.process(cargo_dir().join("cargo")).arg("build"),
+                execs().with_status(0).with_stdout(format!("\
+{updating} registry `[..]`
+{downloading} [..] v0.0.1 (the package registry)
+{downloading} [..] v0.0.1 (the package registry)
+{compiling} baz v0.0.1 (the package registry)
+{compiling} bar v0.0.1 (the package registry)
+{compiling} foo v0.0.1 ({dir})
+", updating = UPDATING, downloading = DOWNLOADING, compiling = COMPILING,
+   dir = p.url()).as_slice()));
+})
+
+test!(relying_on_a_yank_is_bad {
+    let p = project("foo")
+        .file("Cargo.toml", r#"
+            [project]
+            name = "foo"
+            version = "0.0.1"
+            authors = []
+
+            [dependencies]
+            bar = "*"
+        "#)
+        .file("src/main.rs", "fn main() {}");
+    p.build();
+
+    r::mock_pkg("baz", "0.0.1", []);
+    r::mock_pkg_yank("baz", "0.0.2", [], true);
+    r::mock_pkg("bar", "0.0.1", [("baz", "=0.0.2")]);
+
+    assert_that(p.process(cargo_dir().join("cargo")).arg("build"),
+                execs().with_status(101).with_stderr("\
+no package named `baz` found (required by `bar`)
+location searched: the package registry
+version required: = 0.0.2
+"));
+})
+
+test!(yanks_in_lockfiles_are_ok {
+    let p = project("foo")
+        .file("Cargo.toml", r#"
+            [project]
+            name = "foo"
+            version = "0.0.1"
+            authors = []
+
+            [dependencies]
+            bar = "*"
+        "#)
+        .file("src/main.rs", "fn main() {}");
+    p.build();
+
+    r::mock_pkg("bar", "0.0.1", []);
+
+    assert_that(p.process(cargo_dir().join("cargo")).arg("build"),
+                execs().with_status(0));
+
+    fs::rmdir_recursive(&r::registry_path().join("3")).unwrap();
+
+    r::mock_pkg_yank("bar", "0.0.1", [], true);
+
+    assert_that(p.process(cargo_dir().join("cargo")).arg("build"),
+                execs().with_status(0).with_stdout(format!("\
+{updating} registry `[..]`
+", updating = UPDATING).as_slice()));
+
+    assert_that(p.process(cargo_dir().join("cargo")).arg("update"),
+                execs().with_status(101).with_stderr("\
+no package named `bar` found (required by `foo`)
+location searched: the package registry
+version required: *
+"));
+})